-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(db) move consumers from kong.dao to kong.db #3408
Conversation
27a97d9
to
ecf5491
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick first round of review
kong/api/routes/consumers.lua
Outdated
DELETE = function(self, dao_factory) | ||
crud.delete(self.consumer, dao_factory.consumers) | ||
end | ||
}, | ||
|
||
["/consumers/:username_or_id/plugins/"] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be changed to /consumers/:consumers/plugins
for future (not also that no /
at the end)?
|
||
return { | ||
name = "consumers", | ||
primary_key = { "id" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here we need to add endpoint_key = "username",
(see feat/admin-schema-aware-parser)
ecf5491
to
f8cfca1
Compare
f8cfca1
to
d19299c
Compare
kong/api/routes/consumers.lua
Outdated
|
||
["/consumers/:username_or_id/plugins/"] = { | ||
["/consumers/:username_or_id/plugins"] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be /consumers/:consumers/plugins
for future changes (or was it not changed because of a reason?), when plugins are transferred (probably at that point this needs to be removed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed the extra "/". Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also renamed username_or_id
to consumers
on the route.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick note here. -^
536d316
to
211ad2f
Compare
211ad2f
to
adcd198
Compare
Closing this in favour of #3437 |
Moves consumers to the new DAO. Notes:
endpoints.lua
that allow param parsing using schemas (https://github.com/Kong/kong/compare/feat/admin-schema-aware-parser), so that one should probably be merged before this PR.PUT
is gone (for now, @bungle is working on adding it back)/consumers/?created_at=xxx
) is no longer possiblecache_key
should be a DAO method. Maybe it should live in thekong.cache
module instead?cache.key('consumers', consumer_id)
instead ofconsumers:cache_key(consumer_id)
singletons.x